Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mitigate dragon/numpy, mypy/typing_extension dependency issues #653

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

ankona
Copy link
Contributor

@ankona ankona commented Jul 31, 2024

This PR mitigates two issues encountered during installation on build agents

mypy/typing_extensions

Installation of mypy or dragon in separate build actions caused some dependencies (typing_extensions, numpy) to be upgraded. Those upgrades result in runtime failures. The build actions were tweaked to allow pip to consider all optional dependencies during resolution.

dragon/numpy

Additionally, the numpy version was capped on dragon installations.

@ankona ankona requested review from ashao and MattToast July 31, 2024 22:19
Copy link
Member

@ashao ashao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking this one down so quickly

Copy link
Member

@MattToast MattToast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@ankona ankona merged commit 6f6722c into CrayLabs:develop Jul 31, 2024
34 checks passed
Copy link
Member

@MattToast MattToast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing actually! Not worth holding up approval over, though!!

@@ -182,7 +182,7 @@ def install_package(asset_dir: pathlib.Path) -> int:
logger.info(f"Installing package: {wheel_path.absolute()}")

try:
pip("install", "--force-reinstall", str(wheel_path))
pip("install", "--force-reinstall", str(wheel_path), "numpy<2")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually one small note: It looks like the version of numpy that comes in when doing a "normal"

pip install --no-cache-dir smartsim[dev,mypy,ml] && smart build --onnx

is version 1.24.3 (at least for me, from pypi, on osx).

This change will presumably pin to version 1.26.4. I'm not sure if that matters, but if you want to be SUPER safe, you may want to pin to that number.

@MattToast
Copy link
Member

Whoops, just missed the merge, hahaha

@ankona ankona deleted the deps-patch branch September 25, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants